-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add distance to surface tests #224
Conversation
tests/distances/make_ge_gdml.py
Outdated
|
||
|
||
# lar | ||
lar_s = pg4.geant4.solid.Tubs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lar can be removed from this GDML, but then change the mother volume for HPGe to be the world not lar.
detectors = ["V", "P", "B", "C"] | ||
|
||
|
||
n = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these lines are not needed in this case. They only create a macro to simulate in the LAr cylinders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do kinda need both lines. The list of detectors is so i can just loop through all 4 types (although i can omit the detectors variable by just directly inserting the list into the enumerator).
And i need the n to keep track of the unique detector IDs to also create the detectors-fake.mac
to register the detectors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I selected the wrong lines. I meant 70-77 where we create the LAr cylinder information (lines
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, you are right. I dont know how i deleted the write statements below but not that^^
This looks a great test. |
The test fails in CI:
I have found this before, I think you have to import directly the |
The test failed because the pre-commit hooks (namely ruff apparently) removed the |
One other idea. |
I just did some tests:
I could reduce it. The distance in the output file is saved as a double in meters. For double we expect an accuracy of approximately 16 decimals, so we could even go to fm accuracy (although i am not sure of the accuracy in The offset of possibly 5 meters should not be relevant. |
Yeah I'm worried about the global offset more for the distances from |
Ah, i just realized that of course in geant4 we do encounter the same issue. As we do as well use the global coordinates first to then transform them into the local coordinates. And if we lost accuracy, we can not get it magically back using the transform. So forget my previous example^^ In Geant4 doubles are used, so the accuracy would roughly be |
Awesome, thanks Eric. |
Generates a dummy gdml volume consisting of 4 detectors (one of each type) and simulates 1 eV electrons sampled within those volumes. Then checks if the distance output from Geant4 agrees with the calculation from
legendhpges
. Currently the tolerance is implemented as default 1 nm. Additionally produces a plot that shows the hit profile for each detector.Regarding the runtime: the 3 tests added here take combined ~ 10 secs to run on my machine, with all of the now 48 tests (excluding the vis tests) taking ~ 80 secs
Related #223
Edit: Maybe someone have a look over my python code. I copied a lot of stuff from Tobys implementation, but i also changed a lot. I am not that familiar with python so i might have done something in an overcomplicated way